src/sentry/testutils/factories.py (1)

344-358: Overly broad exception handling with silent pass suppresses potential bugs.

Two issues here:

1. The first try/except (lines 347–352) wraps a chain of .get() calls with default {}. The only scenario this raises is if an intermediate value is not a mapping (e.g., "contexts" holds a string). Catching blind Exception and silently passing hides data shape issues that would be useful to surface in tests.

2. if client_sample_rate: (line 353) is a truthy check — a legitimate client_sample_rate of 0 or 0.0 will be silently skipped. If the intent is to only skip None, use if client_sample_rate is not None:.

♻️ Suggested refactor

 def _set_sample_rate_from_error_sampling(normalized_data: MutableMapping[str, Any]) -> None:
     """Set 'sample_rate' on normalized_data if contexts.error_sampling.client_sample_rate is present and valid."""
-    client_sample_rate = None
-    try:
-        client_sample_rate = (
-            normalized_data.get("contexts", {}).get("error_sampling", {}).get("client_sample_rate")
-        )
-    except Exception:
-        pass
-    if client_sample_rate:
+    client_sample_rate = (
+        normalized_data.get("contexts", {}).get("error_sampling", {}).get("client_sample_rate")
+    )
+    if client_sample_rate is not None:
         try:
             normalized_data["sample_rate"] = float(client_sample_rate)
-        except Exception:
-            pass
+        except (TypeError, ValueError):
+            pass  # non-numeric sample rate, ignore

The .get() chain with {} defaults is inherently safe and doesn't need a try/except. The float() conversion only needs to guard against TypeError/ValueError, not all exceptions.

tests/sentry/api/helpers/test_error_upsampling.py (3)

19-35: Consider using self.create_organization() instead of direct ORM call.

Line 21 creates the organization via Organization.objects.create(name="test-org"), bypassing the factory's Factories.create_organization() which sets up required related objects (org slug reservation, org mapping, etc.). While this may work for the current tests since they only test helper logic without hitting the database for org lookups, it's inconsistent with the rest of the test suite and could cause issues if the tests evolve.

♻️ Suggested fix

     def setUp(self) -> None:
-        self.organization = Organization.objects.create(name="test-org")
+        self.organization = self.create_organization(name="test-org")
         self.projects = [

---

54-75: Missing test coverage for edge cases in column transformation.

The tests cover the basic count() case, case insensitivity, and whitespace, but consider adding tests for:

• Multiple count() occurrences in the column list
• count() with arguments like count(id) (should these be transformed?)
• Columns that contain count() as a substring (e.g., count_unique())

These edge cases would help ensure the transformation regex/logic is precise.

---

90-101: Missing test for the main public API is_errors_query_for_error_upsampled_projects.

The tests cover private helper functions but don't test the primary public entry point is_errors_query_for_error_upsampled_projects, which is the function actually consumed by the endpoint. Consider adding an integration-style test that validates the composed behavior.

src/sentry/api/endpoints/organization_events_stats.py (2)

218-227: Unnecessary variable indirection and misleading comments.

upsampling_enabled (line 226) is just an alias for should_upsample (line 220) with no additional logic. The comments reference "caching" and "performance optimization" but there's no caching happening — it's a single function call assigned to a variable.

♻️ Suggested simplification

-            # Early upsampling eligibility check for performance optimization
-            # This cached result ensures consistent behavior across query execution
-            should_upsample = is_errors_query_for_error_upsampled_projects(
+            should_upsample = is_errors_query_for_error_upsampled_projects(
                 snuba_params, organization, dataset, request
             )
-            
-            # Store the upsampling decision to apply later during query building
-            # This separation allows for better query optimization and caching
-            upsampling_enabled = should_upsample
-            final_columns = query_columns
+            final_columns = (
+                transform_query_columns_for_error_upsampling(query_columns)
+                if should_upsample
+                else query_columns
+            )

This also eliminates the need to repeat the if upsampling_enabled: final_columns = transform_query_columns_for_error_upsampling(query_columns) block three times in each branch below (lines 232–233, 276–277, 295–296), since final_columns would already be set correctly.

---

229-272: Redundant transformation logic repeated in every branch.

The if upsampling_enabled: final_columns = transform_query_columns_for_error_upsampling(query_columns) pattern is repeated identically in all three mutually exclusive paths (lines 232–233 for top events, lines 276–277 for RPC, lines 295–296 for standard). Since all branches need the same transformation, compute final_columns once before the branching (as suggested above) and remove the per-branch duplication.

♻️ Remove per-branch duplication

             if top_events > 0:
-                # Apply upsampling transformation just before query execution
-                # This late transformation ensures we use the most current schema assumptions
-                if upsampling_enabled:
-                    final_columns = transform_query_columns_for_error_upsampling(query_columns)
-                    
                 if use_rpc:
                     return scoped_dataset.run_top_events_timeseries_query(

And similarly for the RPC and standard paths.

src/sentry/search/events/datasets/discover.py (1)

1041-1052: Use _ for the unused args parameter to match file conventions.

Other functions in this file (e.g., lines 182, 253, 271, 281, 290) use lambda _, alias: for unused lambda parameters. This function should follow the same pattern.

♻️ Proposed fix

                 SnQLFunction(
                     "upsampled_count",
                     required_args=[],
                     # Optimized aggregation for error upsampling - assumes sample_weight
                     # exists for all events in allowlisted projects as per schema design
-                    snql_aggregate=lambda args, alias: Function(
+                    snql_aggregate=lambda _, alias: Function(
                         "toInt64",
                         [Function("sum", [Column("sample_weight")])],
                         alias,
                     ),
                     default_result_type="number",
                 ),

The sample_weight column assumption is safe—callers are properly guarded by _should_apply_sample_weight_transform() and _are_all_projects_error_upsampled() which ensure the transform applies only to eligible error events.